Add unit tests for spiral shape and arrow shape tools#3958
Add unit tests for spiral shape and arrow shape tools#3958tarone-saloni wants to merge 2 commits intoGraphiteEditor:masterfrom
Conversation
Closes GraphiteEditor#3954 Tests cover all branches of the following pure functions in shape_utility.rs: - wrap_to_tau: zero, positive within range, exactly TAU, beyond TAU, negative values (wraps correctly into [0, 2π)) - format_rounded: trailing-zero trimming, dot trimming, rounding up, zero precision, zero value, all-significant digits retained - calculate_display_angle: positive within range, positive wrapping past 360°/720°, exactly 360°, negative small angle, negative beyond -360°, positive-zero input - arc_end_points_ignore_layer: zero sweep, quarter sweep, half-circle sweep, radius scaling, identity viewport vs. no viewport - star_vertex_position: even index (outer radius), odd index (inner radius), second outer vertex — verifying angle and radius selection math - polygon_vertex_position: vertex 0 (up), vertex 1 of square (right), vertex 2 of square (down) - inside_polygon: center inside, far point outside (bbox early exit), point beyond vertex outside, point near center inside - inside_star: center inside, far point outside (bbox early exit), point beyond outer tip outside, point near center inside
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive suite of unit tests for shape utility functions, covering angle wrapping, coordinate calculations for arcs, stars, and polygons, and hit-testing logic. The feedback identifies opportunities to improve test maintainability and readability by implementing a custom macro for floating-point approximations and utilizing idiomatic vector comparison methods from the glam library.
|
|
||
| #[test] | ||
| fn wrap_pi_stays_pi() { | ||
| assert!((wrap_to_tau(PI) - PI).abs() < 1e-10); |
There was a problem hiding this comment.
There are many floating-point comparisons in these tests using the pattern (a - b).abs() < 1e-10. To improve readability and provide better failure messages, consider defining a local assert_approx_eq! macro at the top of the tests module.
macro_rules! assert_approx_eq {
($a:expr, $b:expr, $eps:expr) => {
assert!(
($a - $b).abs() < $eps,
"assertion failed: `(left - right).abs() < epsilon`\n left: `{:?}`\n right: `{:?}`\nepsilon: `{:?}`",
$a, $b, $eps
);
};
($a:expr, $b:expr) => {
assert_approx_eq!($a, $b, 1e-10);
};
}This would allow you to write this assertion as assert_approx_eq!(wrap_to_tau(PI), PI); and can be used for all scalar float comparisons in this test module, making them more concise and readable.
| assert!((start.x - 1.).abs() < 1e-10, "start.x expected 1, got {}", start.x); | ||
| assert!(start.y.abs() < 1e-10, "start.y expected 0, got {}", start.y); | ||
| assert!((end.x - 1.).abs() < 1e-10, "end.x expected 1, got {}", end.x); | ||
| assert!(end.y.abs() < 1e-10, "end.y expected 0, got {}", end.y); |
There was a problem hiding this comment.
These per-component assertions can be simplified by using glam::DVec2::abs_diff_eq for a more concise and idiomatic vector comparison.
For example, these four lines can be replaced with:
let expected = DVec2::new(1., 0.);
assert!(start.abs_diff_eq(expected, 1e-10), "start point mismatch");
assert!(end.abs_diff_eq(expected, 1e-10), "end point mismatch");This approach can be applied to other vector comparisons in this file (e.g., in arc_endpoints_with_identity_viewport_matches_no_viewport, star_vertex_*, and polygon_vertex_* tests) to make them cleaner and more readable.
Adds #[cfg(test)] coverage to spiral_shape.rs and arrow_shape.rs, following the pattern established in ellipse_shape.rs and line_shape.rs. Spiral tests use raw editor message sends (bypassing handle_message's eval_graph call) because the instrumented graph evaluator does not support SpiralType as a node input. Node inputs are read directly from document.network_interface instead. Tests added: - spiral_draw_simple: outer_radius matches drag distance - spiral_archimedean_inner_radius_default: inner_radius == 0.0 - spiral_logarithmic_inner_radius_default: inner_radius == 0.1 - spiral_cancel_rmb: no layer created on RMB cancel - spiral_default_turns: turns >= 1.0 after draw - arrow_draw_simple: arrow_to matches horizontal drag - arrow_draw_diagonal: arrow_to length matches 3-4-5 drag - arrow_cancel_rmb: no layer created on RMB cancel - arrow_snap_angle_shift: angle snaps to 15° multiple with SHIFT - arrow_zero_length_no_layer: zero-length drag produces no meaningful arrow_to
tarone-saloni
changed the title
1 participant
Summary
Adds
#[cfg(test)]unit test coverage to two currently untested shape tool files, as tracked in #3965.Tests follow the same pattern as
ellipse_shape.rsandline_shape.rs. Node inputs are read directly fromdocument.network_interface(no graph evaluation required), following theline_shape.rsapproach.Note on spiral tests: The spiral tests use raw
editor.editor.handle_message(...)calls instead of theEditorTestUtilshelpers. This is becausehandle_messageinternally callseval_graph(the instrumented graph evaluator), which does not supportSpiralTypeas a node input and would panic. Reading spiral parameters viaextract_spiral_parameters(which queriesdocument.network_interfacedirectly) works without evaluation.Tests added
spiral_shape.rsspiral_draw_simpleouter_radiusmatches drag distance (~40 px)spiral_archimedean_inner_radius_defaultinner_radius == 0.0for Archimedean typespiral_logarithmic_inner_radius_defaultinner_radius == 0.1for Logarithmic typespiral_cancel_rmbspiral_default_turnsturns >= 1.0after a drawarrow_shape.rsarrow_draw_simplearrow_tomatches a horizontal 100 px dragarrow_draw_diagonalarrow_tolength matches a 3-4-5 (60×80→100) dragarrow_cancel_rmbarrow_snap_angle_shiftarrow_zero_length_no_layerarrow_to(guarded by the< 1e-6check)Test plan
cargo test -p graphite-editor spiral— 5 passed, 0 failedcargo test -p graphite-editor arrow— 5 passed, 0 failed